Fix order of operands. #34
Conversation
|
I have also renamed the function for more clarity.
|
src/sail_json_backend/json.ml
Outdated
| try | ||
| let comma_index = String.index operand ',' in | ||
| debug_print ("Operand before trimming: " ^ operand); | ||
| let trimmed = String.sub operand 0 comma_index in | ||
| debug_print ("Final trimmed operand: " ^ trimmed); | ||
| let inputl = Hashtbl.find inputs k in | ||
| if List.mem trimmed inputl then aux (trimmed :: acc) tl else aux acc tl |
There was a problem hiding this comment.
Can you make this a more generic loop that looks at each element in the list of parameters of this function? For example, if it encounters a string "func(a,b,c)", it checks to see if "a", "b", and "c" are in "inputs"?
|
@ThinkOpenly, I made the changes to address your comments. |
src/sail_json_backend/json.ml
Outdated
| let comma_index = String.index operand ',' in | ||
| debug_print ("Operand before trimming: " ^ operand); | ||
| let trimmed = String.sub operand 0 comma_index in | ||
| debug_print ("Final trimmed operand: " ^ trimmed); | ||
| let inputl = Hashtbl.find inputs k in | ||
| if List.mem trimmed inputl then aux (trimmed :: acc) tl else aux acc tl |
There was a problem hiding this comment.
You removed trimmed above, so I'm not sure how you can use it here? :-)
src/sail_json_backend/json.ml
Outdated
| let filter_non_operands components = | ||
| let rec aux acc = function | ||
| | [] -> List.rev acc | ||
| | hd :: tl -> if String.trim hd = "spc" then tl else aux (hd :: acc) tl | ||
| in | ||
| aux [] components |
There was a problem hiding this comment.
I took a stab at an alternative implementation that doesn't use List.rev:
let rec filter_non_operands components = match components with
| [] -> []
| hd :: tl when String.trim hd = "spc" -> tl
| _ -> filter_non_operands (List.tl components)
What do you think? It's slightly smaller, doesn't require an embedded aux function, and doesn't use List.rev, so it could be slightly faster (although we're not terribly concerned with performance at this point).
I think it could be adapted to be used in extract_operands, below, as well.
There was a problem hiding this comment.
It's much more readable. I've adapted it for extact_operands
src/sail_json_backend/json.ml
Outdated
| try | ||
| let inputl = Hashtbl.find inputs k in | ||
| let filtered_elements = List.filter (fun element -> List.mem element inputl) elements in | ||
| filtered_elements @ extract_operands k tl | ||
| with Not_found -> extract_operands k tl |
There was a problem hiding this comment.
Sorry, a little bit "peeling the onion" here. I tend to avoid try blocks if there is a reasonable and straightforward alternative, and there's a bit of redundancy in those last two lines. What do you think about this alternative?
+ let filtered_elements = match Hashtbl.find_opt inputs k with
+ | None -> []
+ | Some inputl -> List.filter (fun element -> List.mem element inputl) elements
+ in filtered_elements @ extract_operands k tl
There was a problem hiding this comment.
Much better, I'll adjust and squash.
src/sail_json_backend/json.ml
Outdated
| let operand = Str.matched_group 1 hd in | ||
| let elements = Str.split (Str.regexp ",") operand in |
There was a problem hiding this comment.
very minor and subjective, but "operand" isn't the best variable name here. Since it is only used in the next line, anyway, let's combine these two lines:
let elements = Str.split (Str.regexp ",") (Str.matched_group 1 hd) in
* Change extraction of assembly operands from AST input list to assembly order. This avoids including extraneous AST inputs in the list of operands, and ensures operands are extracted in the same order as seen in the assembly syntax. * Take care to extract operands embedded within functions, like `funct(op1 @ op2 @ 0b00)`.
This PR addresses the issue described in issue #26 by correcting the order of operands in the JSON output. Previously, operands were extracted from the function clause, but this change extracts them from the right-hand side of the assembly clause. Additionally, non-operand inputs are now filtered out.
To properly match operands to their respective types, the structure of operands stored in the Hashtbl operands has been updated. The previous structure was a simple list of names:
WXTYPE: funct6, vm, vs2, rs1, vdThe new structure stores a list of tuples, each containing a name and its type value:
WXTYPE: (vd, regidx), (vs2, regidx), (rs1, regidx), (vm, bits(1))From the example above note: